test(c/validation): add coverage for option, GetInfo and bind gaps found reviewing adbc-spanner by fornwall · Pull Request #21 · fornwall/arrow-adbc

fornwall · 2026-07-13T15:12:12Z

What this adds

Five new generic, driver-agnostic test cases for the C++ validation suite (c/validation), each staging coverage for a real bug found during a full-project review of the out-of-tree adbc-spanner Rust driver (its REVIEW.md). None of these behaviors were covered by the suite before. All tests are written against the generic DriverQuirks fixtures, so they auto-enroll for every driver using ADBCV_TEST_CONNECTION / ADBCV_TEST_STATEMENT; they are candidates for later upstream submission to apache/arrow-adbc.

Two of the new tests exposed the same bugs in the in-tree SQLite driver, so this PR also carries two small SQLite fixes (first two commits, one per fix) to keep the suite green:

fix(c/driver/sqlite): report the real result schema for a zero-row bound parameter stream — the reader skipped schema inference entirely when the bound parameter stream had zero rows and returned a zero-column schema.
fix(c/driver/sqlite): accept adbc.statement.exec.incremental at its spec default — the option framework rejected even ADBC_OPTION_VALUE_DISABLED (the documented default) with NOT_IMPLEMENTED.

No new quirks were needed; every test passes or self-skips on SQLite via existing quirk guards (supports_get_option(), supports_get_sql_info(), supports_dynamic_parameter_binding()). Commits are one-per-test-area so they can be cherry-picked upstream independently.

Per-test summary

Test	adbc-spanner finding	Spec reference	Expected on current adbc-spanner	How determined
`ConnectionTest.AutocommitIntRoundTrip`	COR-4 (set-as-int succeeds, get-as-int fails)	`adbc.h` doc for `AdbcConnectionGetOptionInt` (c/include/arrow-adbc/adbc.h:1745-1752): "For standard options, drivers must always support getting the option value (if they support getting option values at all) via the type specified in the option. (For example, an option set via SetOptionDouble must be retrievable via GetOptionDouble.)" The test only asserts the typed get when the driver accepted the typed set; drivers that reject `SetOptionInt` (like SQLite) skip.	FAIL — `SetOptionInt(autocommit, 1)` returns OK, then `GetOptionInt` fails `INVALID_ARGUMENT: option adbc.connection.autocommit value "true" is not an integer`	verified by actual run against the driver on the Spanner emulator
`ConnectionTest.MetadataGetInfoAllCodes`	SPEC-5 (`get_info(None)` curated subset vs explicit requests answering more codes)	`adbc.h` doc for `AdbcConnectionGetInfo` (adbc.h:1550-1552): "info_codes A list of metadata codes to fetch, or NULL to fetch all"; and adbc.h:1539-1541: unrecognized requested codes are omitted from the result. A code returned for an explicit request is therefore recognized by the driver, so it must also appear in the NULL/all-codes result.	FAIL — explicit requests for codes 1 (`VENDOR_VERSION`) and 2 (`VENDOR_ARROW_VERSION`) return rows, but the all-codes result is `{0, 3, 4, 100, 101, 102, 103}`	verified by actual run
`StatementTest.SqlBindZeroRows`	COR-9 (zero-row bound batch advertises an empty schema)	No literal spec text mandates this; it is a consistency invariant (stated as such rather than inventing a citation): the result-set schema of a query must not depend on how many parameter rows were bound — a DBAPI `executemany` with an empty parameter list otherwise sees a schema disagreeing with every non-empty execution of the same query.	FAIL — zero-row bind returns a 0-column schema (real result schema has 1 column)	verified by actual run
`StatementTest.SqlBindNullType`	CONV-1 (Null-typed bind columns rejected)	`adbc.h` doc for `AdbcStatementGetParameterSchema` (adbc.h:2284-2286): "If the type cannot be determined, the type of the corresponding field will be NA (NullType)" — a client that builds its bind batch from exactly the schema the driver advertises (or pyarrow inferring `null` for an all-None parameter list) produces NA columns, which the driver must accept.	FAIL — `INVALID_ARGUMENT: cannot bind parameter "p0": unsupported Arrow type Null`	verified by actual run
`StatementTest.SqlQueryIncrementalDefault`	SPEC-2 (`adbc.statement.exec.incremental` rejected even at its spec default)	`adbc.h` doc for `ADBC_STATEMENT_OPTION_INCREMENTAL` (adbc.h:650-665): "The default is ADBC_OPTION_VALUE_DISABLED." The header documents the default rather than literally mandating a set-to-default no-op; the test encodes the practical consequence that a generic client writing back defaults must not break. Actually enabling may still return `NOT_IMPLEMENTED` (asserted).	FAIL — `NOT_IMPLEMENTED: statement option adbc.statement.exec.incremental is not supported by the Spanner ADBC driver` for value `"false"`	verified by actual run

All five adbc-spanner outcomes were verified by an end-to-end run (not derived from source): the harness at adbc-spanner/adbc-validation was built with FETCHCONTENT_SOURCE_DIR_ARROW_ADBC pointed at this branch and run against libadbc_spanner.so (debug build of adbc-spanner main @ c4e1ac9) on the Spanner emulator. All 5 new tests FAILED there with exactly the failure modes predicted by the review findings (messages quoted above).

SQLite verification

cmake -S c -B <build> -DADBC_DRIVER_SQLITE=ON -DADBC_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Debug
cmake --build <build> --target adbc-driver-sqlite-test -j8
<build>/driver/sqlite/adbc-driver-sqlite-test

Full suite at the branch head: 126 passed, 16 skipped, 0 failed (the 15 pre-existing skips plus SqliteConnectionTest.AutocommitIntRoundTrip, which skips because the SQLite driver's option framework rejects integer values for the boolean autocommit option — the intended skip path). A filtered run of the five new tests: 4 pass, 1 skip. Without the two SQLite fix commits, SqlBindZeroRows and SqlQueryIncrementalDefault fail on SQLite with the same bugs found in adbc-spanner.

Touched files were formatted with clang-format 18.1.7 (the version pinned in .pre-commit-config.yaml).

Note: the new tests also auto-enroll in the other in-tree driver test suites (e.g. PostgreSQL, which needs a live server); those were not run here since only the SQLite reference driver was in scope.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

…und parameter stream When a query was executed with a bound parameter stream containing zero rows, the reader skipped schema inference entirely and reported an empty (zero-column) schema instead of the statement's actual result schema. Run InferFinalize even when the binder finishes before the first execution, so the schema has the correct number of columns (matching the behavior of an ordinary query returning zero rows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

…pec default ADBC_STATEMENT_OPTION_INCREMENTAL defaults to disabled (adbc.h), so a generic client that writes back option defaults must be able to set it to ADBC_OPTION_VALUE_DISABLED as a no-op. Keep returning NOT_IMPLEMENTED when a caller actually tries to enable incremental execution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

If a driver accepts AdbcConnectionSetOptionInt for the standard autocommit option, AdbcConnectionGetOptionInt on the same key must return the value that was set: per adbc.h, drivers must support getting an option value via the type it was set with. Drivers that reject the integer set (like the SQLite driver, whose option framework only parses "true"/"false" strings for booleans) skip cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

… requested codes AdbcConnectionGetInfo with NULL info_codes fetches all metadata the driver recognizes (adbc.h). A driver that returns a row for an explicitly requested info code therefore recognizes that code, so the same code must also appear in the all-codes result; otherwise the two forms disagree about what the driver supports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

…al result schema Executing a parameterized query with a bound parameter batch of zero rows (e.g. a DBAPI executemany with an empty parameter list) must return a result stream whose schema matches the schema of any non-empty execution of the same query, with zero rows - not an empty (zero column) schema. This is a consistency invariant rather than literal spec text; requires the preceding SQLite reader fix to pass there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

…s accepted AdbcStatementGetParameterSchema reports parameters whose type cannot be determined as NA (adbc.h), and clients like pyarrow infer a null-typed column for an all-None parameter list. A driver must therefore accept a bind batch containing a null-typed column, treating every value as NULL. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

…spec default ADBC_STATEMENT_OPTION_INCREMENTAL has a documented default of ADBC_OPTION_VALUE_DISABLED (adbc.h), so setting it to that value must succeed as a no-op even in drivers without incremental execution; generic clients (which may write back defaults unconditionally) break otherwise. Enabling it may still return NOT_IMPLEMENTED, and a plain query must keep working afterwards. Requires the preceding SQLite option fix to pass there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LGf8PVEe2tYkw8Q6Pd95tq

fornwall · 2026-07-13T17:00:56Z

Split into five independent PRs, one per validation test (with each SQLite fix riding along with the test that exposed it):

test(c/validation): assert typed option set/get coherence for autocommit #22 — ConnectionTest.AutocommitIntRoundTrip (typed option set/get coherence)
test(c/validation): assert GetInfo all-codes result covers explicitly requested codes #23 — ConnectionTest.MetadataGetInfoAllCodes (GetInfo all-codes covers explicitly requested codes)
test(c/validation): assert a zero-row bound batch still yields a result schema #24 — StatementTest.SqlBindZeroRows + SQLite zero-row bound stream schema fix
test(c/validation): assert a null-typed (NA) bound parameter column is accepted #25 — StatementTest.SqlBindNullType (null-typed bound parameter column)
test(c/validation): assert the incremental option is accepted at its spec default #26 — StatementTest.SqlQueryIncrementalDefault + SQLite incremental-option default fix

Closing in favor of those.

fornwall and others added 7 commits July 13, 2026 17:03

fornwall closed this Jul 13, 2026

This was referenced Jul 13, 2026

test(query): executing with an empty bound batch keeps the result schema fornwall/validation#7

Open

test: null/dictionary bind columns, empty bound batch schema, int option coherence fornwall/validation#4

Closed

Navigation Menu

test(c/validation): add coverage for option, GetInfo and bind gaps found reviewing adbc-spanner - #21

test(c/validation): add coverage for option, GetInfo and bind gaps found reviewing adbc-spanner#21
fornwall wants to merge 7 commits into
mainfrom
validation/review-gap-tests

fornwall commented Jul 13, 2026

Uh oh!

fornwall commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

fornwall commented Jul 13, 2026

What this adds

Per-test summary

SQLite verification

Uh oh!

fornwall commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant